fix: unreachable LDAP backend must not block local/DB account login#62220
fix: unreachable LDAP backend must not block local/DB account login#62220mvanhorn wants to merge 2 commits into
Conversation
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
There was a problem hiding this comment.
| $this->logger->warning('Unable to check password against user backend:' . $backend instanceof IUserBackend ? $backend->getBackendName() : get_class($backend), ['exception' => $e]); |
There was a problem hiding this comment.
| $this->logger->warning('Unable to check password against user backend:' . $backend instanceof IUserBackend ? $backend->getBackendName() : get_class($backend), ['exception' => $e]); |
There was a problem hiding this comment.
Need to be adapted too afterward
Address review: log which backend was unreachable in the checkPasswordNoLogging warnings, using getBackendName() for IUserBackend backends and get_class() otherwise. Parenthesize the ternary so the concatenation and instanceof do not bind ahead of it. Adapt ManagerTest to stub getBackendName() and expect the backend name in the message. Signed-off-by: mvanhorn <mvanhorn@gmail.com>
|
Added in 18c7c67, with one adjustment to the snippet: I wrapped the ternary in parentheses, since . and instanceof bind tighter than ?:, so |
2 participants
Summary
In
OC\User\Manager::checkPasswordNoLogging()the twoforeach ($backends ...)loops call$backend->checkPassword()directly; when the LDAP backend (OCA\User_LDAP\User_Proxy) cannot reach the server it throwsOC\ServerNotAvailableException, which propagates uncaught out ofSession::loginWithPassword()(the login command chain catches only Session/Token exceptions), surfacing as the reported 500 and preventing any later backend (the localOC\User\Databasebackend that would authenticate the local admin) from being tried. Wrap each per-backendcheckPassword()call in atry/catch (\OC\ServerNotAvailableException $e)that logs a warning via the injected logger andcontinues to the next backend, so an unavailable backend degrades gracefully instead of blocking login for users of other backends. The exception is logged (not silently swallowed) to preserve diagnosability, and credential validation for every backend is unchanged, so this is a purely defensive hardening with no authentication-bypass surface. Adduse OC\ServerNotAvailableException;(or fully-qualify) sinceManagerlives in theOC\Usernamespace.Checklist
AI (if applicable)